feat: make auto-recall query length limit configurable (autoRecallMaxQueryLength)#374
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c0ab0ba0ca
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| // FR-04: Truncate long prompts (e.g. file attachments) before embedding. | ||
| // Auto-recall only needs the user's intent, not full attachment text. | ||
| const MAX_RECALL_QUERY_LENGTH = config.autoRecallMaxQueryLength ?? 2_000; |
There was a problem hiding this comment.
Wire new query-length setting into parsed config
config.autoRecallMaxQueryLength is read here, but parsePluginConfig never copies cfg.autoRecallMaxQueryLength into the returned PluginConfig, so this value is always undefined at runtime and the code always falls back to 2_000. In practice, users cannot actually configure the limit despite the commit message and inline docs saying they can.
Useful? React with 👍 / 👎.
AliceLJY
left a comment
There was a problem hiding this comment.
LGTM — changes are clean, on-topic, and well-tested. Approving.
Conflict AnalysisThis PR has a base that's 7 commits behind
The Proposed fix approach:
This would be a straightforward rebase + re-apply. @xiaoyuervae would you like help with the rebase? I can handle it on this PR if you'd like. |
|
Hey @jlin53882, thanks for this — truncating long prompts before embedding is a solid improvement for auto-recall quality. Your conflict analysis is spot-on. The main blocker right now is the merge conflict with master (the
These are needed so the config is discoverable via schema validation and any UI that reads the plugin schema. Once you've rebased and added the config plumbing, happy to re-review! Thanks for the thorough conflict diagnosis. 🙏 |
…lMaxQueryLength (default 2000) - Add FR-04 truncation before embedding to keep query focused - Make MAX_RECALL_QUERY_LENGTH configurable via config.autoRecallMaxQueryLength - Default to 2000 chars (previously hardcoded 1000 in some builds) - Add info log when truncation occurs for visibility - Mirrors existing config pattern (autoRecallMaxItems, autoRecallMaxChars, etc.)
c0ab0ba to
217a6ef
Compare
|
Hi @AliceLJY — PR has been rebased onto latest master and all conflicts resolved. Changes:
Merge state: ✅ CLEAN / MERGEABLE Ready for re-review 🙏 |
Summary
Make the auto-recall query length limit configurable via
config.autoRecallMaxQueryLength, defaulting to 2000 chars.Problem
When auto-recall is enabled, the full
event.prompt(which may include long file attachment descriptions, conversation metadata, or multi-turn context) is passed directly as the embedding query. This can dilute the semantic signal and produce lower-quality retrieval results.Solution
Add FR-04 truncation logic before embedding, configurable via
config.autoRecallMaxQueryLength:Design notes
autoRecallMaxItems,autoRecallMaxChars,autoRecallPerItemMaxCharsall follow the sameconfig.X ?? defaultpatternRelated